Skip to content

fix(formatter/sort-imports): hard line inside multiline import leads to incorrect results#17880

Merged
leaysgur merged 1 commit intooxc-project:mainfrom
nilptr:nilptr/oxfmt/fix/hard-line-in-multiline-import
Jan 14, 2026
Merged

fix(formatter/sort-imports): hard line inside multiline import leads to incorrect results#17880
leaysgur merged 1 commit intooxc-project:mainfrom
nilptr:nilptr/oxfmt/fix/hard-line-in-multiline-import

Conversation

@nilptr
Copy link
Contributor

@nilptr nilptr commented Jan 11, 2026 •
edited
Loading

This PR fixes a minor issue mentioned in #17576.

image

@nilptr nilptr requested a review from Dunqing as a code owner January 11, 2026 16:00
@github-actions github-actions bot added A-formatter Area - Formatter C-bug Category - Bug labels Jan 11, 2026
@codspeed-hq
Copy link

codspeed-hq bot commented Jan 11, 2026 •
edited
Loading

CodSpeed Performance Report

Merging this PR will not alter performance

Comparing nilptr:nilptr/oxfmt/fix/hard-line-in-multiline-import (a763a80) with main (06288ef)1

Summary

✅ 38 untouched benchmarks
⏩ 7 skipped benchmarks2

Footnotes

  1. No successful run was found on main (e384c49) during the generation of this report, so 06288ef was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

  2. 7 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@nilptr nilptr force-pushed the nilptr/oxfmt/fix/hard-line-in-multiline-import branch from eecfcfa to 1291ea9 Compare January 12, 2026 01:19
leaysgur
leaysgur requested changes Jan 13, 2026
View reviewed changes
Copy link
Member

@leaysgur leaysgur left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you please:

  • double check this fix is really needed?
    • I recall having made a similar fixes recently
  • add test cases for this?
    • All tests are in crates/oxc_formatter/tests/ir_transform/sort_imports.rs.

@nilptr nilptr force-pushed the nilptr/oxfmt/fix/hard-line-in-multiline-import branch from 1291ea9 to a763a80 Compare January 13, 2026 15:23
@nilptr
Copy link
Contributor Author

nilptr commented Jan 13, 2026

sorry, I copied the markdown from the description, didn't notice the image is broken.

  1. Yes, it's needed. Your fixes handle the multiline comment, while this fix focus on the comment inside a multiline import.
  2. added a test case

leaysgur
leaysgur approved these changes Jan 14, 2026
View reviewed changes
Copy link
Member

@leaysgur leaysgur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@leaysgur leaysgur merged commit bbadb8e into oxc-project:main Jan 14, 2026
27 checks passed
@leaysgur leaysgur changed the title fix(oxfmt/sort-imports): hard line inside multiline import leads to i… fix(formatter/sort-imports): hard line inside multiline import leads to i… Jan 14, 2026
@leaysgur leaysgur changed the title fix(formatter/sort-imports): hard line inside multiline import leads to i… fix(formatter/sort-imports): hard line inside multiline import leads to incorrect results Jan 14, 2026
@Boshen Boshen mentioned this pull request Jan 19, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@leaysgur leaysgur leaysgur approved these changes

@Dunqing Dunqing Awaiting requested review from Dunqing Dunqing is a code owner

Assignees

@leaysgur leaysgur

Labels

A-formatter Area - Formatter C-bug Category - Bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nilptr @leaysgur